Add createImageUrl utility function to api app#123
Open
femincan wants to merge 6 commits intoplayfulprogramming:mainfrom
Open
Add createImageUrl utility function to api app#123femincan wants to merge 6 commits intoplayfulprogramming:mainfrom
createImageUrl utility function to api app#123femincan wants to merge 6 commits intoplayfulprogramming:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a shared Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/utils.ts`:
- Around line 4-5: The S3 URL builder currently concatenates env.S3_PUBLIC_URL
and env.S3_BUCKET into s3PublicUrl and then calls new URL(path, s3PublicUrl),
which can produce double slashes or drop the bucket when inputs contain extra
slashes; update the logic that builds s3PublicUrl and prepares path by trimming
any trailing '/' from env.S3_PUBLIC_URL and any leading '/' from the incoming
path (and ensure exactly one '/' between S3_PUBLIC_URL and S3_BUCKET), then call
new URL(cleanedPath, cleanedBase) (refer to s3PublicUrl and the new URL(path,
s3PublicUrl) call) so resolved object URLs are always correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83f2c835-5a4a-4526-a907-abd24833ee76
📒 Files selected for processing (4)
apps/api/src/routes/content/collections.tsapps/api/src/routes/tasks/post-images.tsapps/api/src/routes/tasks/url-metadata.tsapps/api/src/utils.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/api/src/utils.ts (1)
9-10:⚠️ Potential issue | 🟠 MajorNormalize the base URL and key before resolving.
This still has the previously reported edge case: if Line 9 receives a trailing
/inenv.S3_PUBLIC_URLor Line 10 receives a leading/inpath,new URL()can either emit malformed URLs or resolve from the CDN root and dropenv.S3_BUCKET.Suggested fix
export function createImageUrl(path: string): string { - const s3PublicUrl = `${env.S3_PUBLIC_URL}/${env.S3_BUCKET}/`; - return new URL(path, s3PublicUrl).toString(); + const baseUrl = `${env.S3_PUBLIC_URL.replace(/\/+$/, "")}/${env.S3_BUCKET.replace( + /^\/+|\/+$/g, + "", + )}/`; + const normalizedPath = path.replace(/^\/+/, ""); + return new URL(normalizedPath, baseUrl).toString(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/utils.ts` around lines 9 - 10, The s3PublicUrl and path must be normalized before calling new URL to avoid losing the bucket when env.S3_PUBLIC_URL has a trailing slash or path has a leading slash; trim trailing slashes from env.S3_PUBLIC_URL and trim leading slashes from path (or build a base as `${trimmedPublicUrl}/${trimmedBucket}/`) then call new URL(cleanPath, cleanBase). Use the existing s3PublicUrl, env.S3_PUBLIC_URL, env.S3_BUCKET and the path variable names to locate and update the construction logic around the new URL(...) call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/api/src/utils.ts`:
- Around line 9-10: The s3PublicUrl and path must be normalized before calling
new URL to avoid losing the bucket when env.S3_PUBLIC_URL has a trailing slash
or path has a leading slash; trim trailing slashes from env.S3_PUBLIC_URL and
trim leading slashes from path (or build a base as
`${trimmedPublicUrl}/${trimmedBucket}/`) then call new URL(cleanPath,
cleanBase). Use the existing s3PublicUrl, env.S3_PUBLIC_URL, env.S3_BUCKET and
the path variable names to locate and update the construction logic around the
new URL(...) call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 963512ec-07a4-45bc-a53a-c018eec37c0b
📒 Files selected for processing (1)
apps/api/src/utils.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces new
createImageUrlutility function to api app to prevent duplicated inline image url creation in api routes.Summary by CodeRabbit